Add missing theme sub args#6492
Conversation
| #' @export | ||
| #' @describeIn subtheme Theme specification for both y axes. | ||
| theme_sub_axis_y <- function(title, text, ticks, ticks.length, line) { | ||
| theme_sub_axis_y <- function(title, text, ticks, ticks.length, line, minor.ticks.length) { |
There was a problem hiding this comment.
Why not minor.ticks argument for this and above?
There was a problem hiding this comment.
There is no theme(axis.minor.ticks), they all descent from the major ticks.
There was a problem hiding this comment.
but then why do you have them below?
There was a problem hiding this comment.
I should've been more specific: they descent from position-resolved major ticks. To illustrate: there is no minor.ticks.y because minor.ticks.y.left and minor.ticks.y.right descent from ticks.y.left and ticks.y.right respectively.
There was a problem hiding this comment.
I still find this weird but I trust your judgement
There was a problem hiding this comment.
I don't disagree with you, it is just the least-worst option I think.
We deliberated in #5287 (comment) wether inheritance should be like this:
axis.ticks
/ \
axis.ticks.x axis.ticks.minor
| |
axis.ticks.x.top axis.ticks.minor.x
|
axis.ticks.minor.x.top
or like this:
axis.ticks
|
axis.ticks.x
|
axis.ticks.x.top
|
axis.ticks.x.top.minor
Both are valid but eventually we chose option 2, with the minor ticks inheriting from the most specific major ticks,
That is why there is no axis.ticks.x.minor and axis.ticks.minor theme elements.
I'm happy to entertain alternative parentage for the minor ticks, but that is something we can delay until after release.
|
Thanks for the review! |
2 participants
This PR aims to fix #6491.
It integrates the
panel.widths,panel.heights,axis.minor.ticks.length.{...}andaxis.minor.ticks.{...}settings in thetheme_sub_axis_*()family andtheme_sub_panel()function.A demo:
Created on 2025-06-02 with reprex v2.1.1